Fix doShutdown() task counting - #73
Conversation
af409a1 to
2cdaa55
Compare
| } | ||
| } | ||
|
|
||
| exec.shutdown(); |
There was a problem hiding this comment.
While unlikely what happens if svc.take throws InterruptedException ? Should exec.shutdown/time.cancel be in a finally block?
There was a problem hiding this comment.
Nice catch! In fact, we should put it all in a try and move that to finally. Also I noticed a catch ( InterruptedException e ) without Thread.currentThread().interrupt(); looking now.
There was a problem hiding this comment.
Changes made in 8475807, but I'll need to rebase. Code is the same just moved to try/finally.
| { | ||
| svc.submit( () -> shutdownAction.apply( tracker ) ); | ||
| submitted++; | ||
| } |
There was a problem hiding this comment.
While both work, was there a reason to use the for loop instead of streams? i.e. instead of
int submitted = cache.size();
cache.values().forEach( tracker -> svc.submit( () -> shutdownAction.apply( tracker ) ) );
There was a problem hiding this comment.
Fair. It should be the same. The only reason I changed it was to count submitted, which should be equal to the cache.size().
There was a problem hiding this comment.
Fixed in adee2f4. Need to rebase, though.
The `doShutdown()` method counted the number of threads created rather than the number of tasks submitted. It was also off by one when draining.
adee2f4 to
c179c85
Compare
2 participants
The
doShutdown()method counted the number of threads created rather than the number of tasks submitted. It was also off by one when draining.